Skip to content

Conversation

@aditya520
Copy link
Member

Summary

Rationale

How has this been tested?

  • Current tests cover my changes
  • Added new tests
  • Manually tested the code

@vercel
Copy link

vercel bot commented Oct 15, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
api-reference Ready Ready Preview Comment Oct 29, 2025 9:18pm
component-library Ready Ready Preview Comment Oct 29, 2025 9:18pm
developer-hub Ready Ready Preview Comment Oct 29, 2025 9:18pm
entropy-explorer Ready Ready Preview Comment Oct 29, 2025 9:18pm
insights Ready Ready Preview Comment Oct 29, 2025 9:18pm
proposals Ready Ready Preview Comment Oct 29, 2025 9:18pm
staking Ready Ready Preview Comment Oct 29, 2025 9:18pm

Copy link
Contributor

@tejasbadadare tejasbadadare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing logic LGTM! Would be great to see some positive and negative tests

Comment on lines 249 to 253
function hasPrice(
PythLazerStructs.Feed memory feed
) public pure returns (bool) {
return (feed.existsFlags & PythLazerStructs.PRICE_EXISTS) != 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we create getters for the properties that enforce checking existence? It would ensure users dont assume a property like price always exists. Something like getPrice(feed) -> (bool, int64), it could return (exists, value). It's an alternative to storing the type as Option<Option<PropertyType>>.

Copy link
Member Author

@aditya520 aditya520 Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are chances that user can ignore the exist flag. Plus returning a bool will consume more gas.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it stands they would have to first call hasPrice before using the price, which to me sounds like a greater chance of being missed by implementors since they would have to know about that method and intentionally use it. On the other hand, the exists flag more clearly signals the possibility that the field may not exist, and one would have to acknowledge this to ignore it. But that's just my opinion

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can just call getPrice(). It will return an error if price is zero.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants